-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: PriorityRpc
, RegisterProvider
, and ManageCanister
caller authorizations
#122
Conversation
@@ -30,7 +30,7 @@ pub fn require_register_provider() -> Result<(), String> { | |||
} | |||
|
|||
pub fn is_rpc_allowed(caller: &Principal) -> bool { | |||
METADATA.with(|m| m.borrow().get().open_rpc_access) || is_authorized(caller, Auth::Rpc) | |||
METADATA.with(|m| m.borrow().get().open_rpc_access) || is_authorized(caller, Auth::PriorityRpc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was supposed to be named PriorityRpc
instead of Rpc
, but it seems like this change was lost in some unmerged PR a while ago.
@@ -14,7 +14,7 @@ pub fn is_authorized(principal: &Principal, auth: Auth) -> bool { | |||
|
|||
pub fn require_admin_or_controller() -> Result<(), String> { | |||
let caller = ic_cdk::caller(); | |||
if is_authorized(&caller, Auth::ManageService) || ic_cdk::api::is_controller(&caller) { | |||
if is_authorized(&caller, Auth::ManageCanister) || ic_cdk::api::is_controller(&caller) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renaming ManageService
to ManageCanister
for clarity. In all other cases, the term "service" refers to third-party RPC services rather than the EVM RPC canister itself. Updated the design document accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ManageCanister
could be confused with the management canister (or a management canister function), so this name is also not idea...
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I see what you're saying. Maybe just Manage
then? I'll do this refactor in a separate PR so we can try a few different options.
PriorityRpc
and RegisterProvider
principal authorizationsPriorityRpc
, RegisterProvider
, and ManageCanister
caller authorizations
self.call_update("registerProvider", Encode!(&args).unwrap()) | ||
.wait() | ||
} | ||
|
||
pub fn unregister_provider(&self, provider_id: u64) -> bool { | ||
self.call_update("unregisterProvider", Encode!(&provider_id).unwrap()) | ||
.wait() | ||
} | ||
|
||
pub fn update_provider(&self, args: UpdateProviderArgs) { | ||
self.call_update("updateProvider", Encode!(&args).unwrap()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume panicking in these functions is okay, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep; these functions are only used by us to manage the canister.
No description provided.